Skip to content

added WorkflowServiceGrpc and IWorkflowServiceV4 #999

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: v4.x.x
Choose a base branch
from

Conversation

shijiesheng
Copy link
Member

@shijiesheng shijiesheng commented May 15, 2025

What changed?

  • added client entities that maps 1:1 with thrift (generated from a generator)
  • added BaseError, which is a catchall similar to TException.
  • added mappers to transform Proto to client entities. (mostly copied from thrift mappers)
  • added IWorkflowServiceV4 which is exactly same as IWorkflowService except for it's using the client entities.
  • added WorkflowServiceGrpc that implements the V4 interface

Plan

  • new V4 interface with new entities < This PR
  • replace all pointers to V4 in one go (Should be import change only)

Why?

Thrift deprecation work

How did you test it?

Unit Test on mappers from ClientObjects to ProtoObjects

Potential risks

Release notes

Documentation Changes

@dkrotx
Copy link
Member

dkrotx commented May 26, 2025

Is this a complete switch to GRPC, so we're throwing away thrift?
I wonder how much of the diff is auto-generated and with one worth checking? the diff looks rather huge.

@shijiesheng
Copy link
Member Author

Is this a complete switch to GRPC, so we're throwing away thrift? I wonder how much of the diff is auto-generated and with one worth checking? the diff looks rather huge.

I finally get the chance to add the unit test for mappers.

To answer your question, I mainly need reviews on 1:1 comparison of thrift vs newly added entities. Others are captured by mappers tests.

build.gradle Outdated
@@ -89,6 +89,11 @@ dependencies {
compile group: 'com.google.api.grpc', name: 'proto-google-common-protos', version: '2.10.0'
compile group: 'com.google.protobuf', name: 'protobuf-java-util', version: '3.21.9'
compile group: 'com.google.oauth-client', name: 'google-oauth-client', version: '1.35.0'
compile group: 'org.projectlombok', name: 'lombok', version: '1.18.30'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only need the compileOnly dependency. We don't want to include this at runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants